Skip to content

Revert torch-family pin centralization#19334

Merged
JacobSzwejbka merged 1 commit intomainfrom
revert-torch-pin-consolidation
May 6, 2026
Merged

Revert torch-family pin centralization#19334
JacobSzwejbka merged 1 commit intomainfrom
revert-torch-pin-consolidation

Conversation

@JacobSzwejbka
Copy link
Copy Markdown
Contributor

Several macos jobs have been timing out really bad since this change blocking viablestrict. We could potentially just increase the runtime but until I actually get a viable strict bump I dont want to be sat waiting 1-2hrs for these jobs to run so reverting to get back to the 30-40m runtimes.

@JacobSzwejbka JacobSzwejbka requested a review from lucylq as a code owner May 6, 2026 17:23
@pytorch-bot pytorch-bot Bot added the ci-no-td label May 6, 2026
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented May 6, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19334

Note: Links to docs will display an error until the docs builds have been completed.

⏳ No Failures, 247 Pending

As of commit 1d2bb89 with merge base 1debeb6 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@JacobSzwejbka JacobSzwejbka merged commit 3ffaf27 into main May 6, 2026
340 of 502 checks passed
@JacobSzwejbka JacobSzwejbka deleted the revert-torch-pin-consolidation branch May 6, 2026 17:36
rascani added a commit that referenced this pull request May 8, 2026
### Summary

`setup-macos.sh` runs `install_pip_dependencies` before
`install_pytorch_and_domains`. That order lets torchsr's transitive
torch dep get pulled from PyPI before the pinned source-built or
S3-cached torch lands; `install_pytorch_and_domains` then overwrites the
wrong-source torch.

The overwrite is small in the current state, but the same race forced
--no-cache-dir`. That cascaded reinstalls of every torch transitive dep,
pushed the macOS unittest past its 60-minute timeout, until #19334
reverted it.

Reorder so `install_pip_dependencies` runs after
`install_pytorch_and_domains`. With torch already at the pinned version,
torchsr's dep is satisfied and pip skips re-downloading. Removes the
structural footgun so any future re-land of centralized torch install
does not need `--force-reinstall`.

Also rewrite two adjacent comments:
- Add a comment above `install_pytorch_and_domains` recording the
ordering rationale.
- Drop the now-misleading "We build PyTorch from source here" comment
that drifted above `install_executorch`; replace with one explaining why
`install_executorch --use-pt-pinned-commit` is correct (torch is already
installed).

Authored with Claude Code.

### Test plan
CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-td CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants